Skip to content

refactor!: drop pkg-CLI mirror inputs, own only action-layer surface#13

Merged
robertsLando merged 11 commits intomainfrom
refactor/slim-pkg-inputs
Apr 24, 2026
Merged

refactor!: drop pkg-CLI mirror inputs, own only action-layer surface#13
robertsLando merged 11 commits intomainfrom
refactor/slim-pkg-inputs

Conversation

@robertsLando
Copy link
Copy Markdown
Member

@robertsLando robertsLando commented Apr 23, 2026

Summary

  • Drops 11 pkg-CLI mirror inputs from the action surface: mode, node-version, compress-node, fallback-to-source, public, public-packages, options, no-bytecode, no-dict, debug, extra-args. Users express these in their pkg config (.pkgrc, pkg.config.{js,ts,json}, or the pkg field of package.json); buildPkgArgs forwards the config path to pkg via --config.
  • Keeps only action-layer inputs — concerns pkg config cannot express: config, entry, targets, pkg-version/pkg-path, archive (strip/compress/filename/checksum), Windows-metadata resedit, macOS/Windows signing, cache, step summary.
  • Docs refreshed: README.md gains a "pkg configuration" migration section with an example; docs/architecture.md §3/§6 describe the scoped surface and rationale; STATUS.yaml records the dropped list and migration path; docs/inputs.md regenerated.

Motivation

Mirroring pkg's CLI meant every new pkg flag forced a back-compat-preserving input bump here and churn in buildPkgArgs / INPUT_SPECS. Decoupling the action from pkg's CLI evolution is cleaner: pkg owns its schema, this action owns CI-layer concerns.

Breaking change

Setting any dropped input now triggers the existing unknown-input warning and the value is ignored. Migration: move the value into the pkg config file. Example:

// .pkgrc.json
{ "bin": "src/main.js", "mode": "sea", "compressNode": "Brotli", "fallbackToSource": true }
- uses: yao-pkg/pkg-action@v1
  with:
    config: .pkgrc.json
    targets: node22-linux-x64

Diff shape

  • packages/core/src/inputs.ts: INPUT_SPECS and BuildInputs slimmed; parser block reduced to 5 fields.
  • packages/core/src/pkg-runner.ts: buildPkgArgs emits only --targets, --config, --out-path, entry. No pkg-flag synthesis.
  • packages/core/src/targets.ts: two cross-compile warnings rephrased to reference pkg config fallbackToSource instead of the retired --fallback-to-source input.
  • scripts/gen-action-yml.ts: composite cache key no longer references inputs.node-version; key now depends on runner.os/runner.arch + hashFiles('**/package.json', '.pkgrc*', '**/pkg.config.{js,ts,json}').
  • Regenerated: action.yml, packages/build/action.yml, docs/inputs.md.
  • Rebuilt: packages/{build,matrix,windows-metadata}/dist/*.mjs.
  • Tests: inputs.test.ts removes assertions for dropped fields, adds a test asserting the dropped inputs are flagged as unknown; pkg-runner.test.ts adds PKG_FLAGS_OWNED_BY_CONFIG guard asserting those flags are never synthesized into argv.

Net -300 LOC.

Test plan

  • yarn typecheck — green
  • yarn lint — green (ESLint + Prettier)
  • yarn test — 222/222 pass, coverage report emitted
  • yarn build — all four dists rebuilt
  • yarn gen — action.yml + docs/inputs.md regenerated, diff committed
  • CI: codegen-drift / bundle-drift gates on the PR
  • E2E: tiny-app-{cjs,esm} + matrix plan→fanout + windows-metadata jobs

Pkg-specific knobs (mode, node-version, compress-node, fallback-to-source,
public, public-packages, options, no-bytecode, no-dict, debug, extra-args)
are removed from the action input surface. Users express them in their pkg
config (.pkgrc, pkg.config.{js,ts,json}, or package.json pkg field) instead.
The action keeps only action-layer concerns that do not exist in pkg config:
config path, entry, targets, pkg-version/pkg-path, plus the archive, signing,
windows-metadata, and performance groups.

Motivation: mirroring pkg's CLI forced the action to grow a new input each
time pkg added a flag, and to maintain back-compat on every pkg release. By
letting pkg own its own schema we decouple the action from pkg's evolution.

Docs updated: README gets a new "pkg configuration" section with a migration
example; architecture.md §3/§6 note the scoped surface; STATUS.yaml records
the dropped-inputs list and migration path.

BREAKING CHANGE: the listed inputs are no longer accepted. Setting them now
triggers the unknown-input warning and the value is ignored. Migrate into
your pkg config file.
Copilot AI review requested due to automatic review settings April 23, 2026 09:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the action to stop mirroring pkg CLI flags as GitHub Action inputs, keeping only action-layer concerns (targets/config/install/signing/archive/cache/summary) and delegating pkg-specific behavior to the user’s pkg config file via --config.

Changes:

  • Slims INPUT_SPECS/BuildInputs and input parsing to remove 11 pkg-CLI mirror inputs.
  • Simplifies buildPkgArgs to emit only --targets, --config, --out-path, and the entry.
  • Regenerates action metadata/docs/bundles and updates tests to enforce “no pkg-flag synthesis”.

Reviewed changes

Copilot reviewed 12 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/gen-action-yml.ts Updates generated composite cache-key to remove dependency on removed node-version input.
action.yml Drops removed inputs and their passthrough; updates cache-key accordingly.
packages/build/action.yml Drops removed inputs from the internal build action surface.
packages/core/src/inputs.ts Removes pkg-CLI mirror inputs from the single source of truth and from parsing/types.
packages/core/src/pkg-runner.ts Stops synthesizing pkg CLI flags; forwards only targets/config/out-path/entry.
packages/core/src/targets.ts Rephrases cross-compile warnings to point users to pkg config for fallback behavior.
packages/core/test/unit/inputs.test.ts Removes assertions for dropped inputs; adds unknown-input coverage for removed fields.
packages/core/test/unit/pkg-runner.test.ts Adds guard test ensuring removed pkg-layer flags are never emitted into argv.
docs/inputs.md Regenerated input reference without the dropped inputs.
docs/architecture.md Documents the narrowed input-surface scope and rationale.
README.md Adds migration guidance section explaining pkg config ownership.
STATUS.yaml Records the breaking input-surface change and migration path.
packages/build/dist/index.mjs Rebuilt bundle reflecting new input surface and argv construction.
packages/matrix/dist/index.mjs Rebuilt bundle reflecting updated warning text.
packages/windows-metadata/dist/index.mjs Rebuilt bundle reflecting new input surface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/targets.ts Outdated
Comment thread packages/core/src/targets.ts Outdated
Comment thread README.md Outdated
…tests

Address PR-review suggestions:
- architecture.md §6 no longer duplicates the dropped-input list; points
  readers at STATUS.yaml#input-surface-slim as the single source.
- inputs.test.ts: extend the defaults test with `pkgPath` and add a positive
  round-trip test for `pkg-version` + `pkg-path`. These remain live inputs and
  deserve the same explicit coverage the rest of BuildInputs has.

223/223 unit tests, lint clean.
New optional input `config-inline` accepts a JSON string carrying the pkg
config directly in the workflow. The orchestrator writes it to
`${invocationDir}/pkg-config.inline.json` and forwards that path to pkg via
`--config`. Mutually exclusive with `config`; the pair is validated at parse
time alongside a JSON-object syntax check so a typo fails before pkg runs.

Motivation: users with a trivial pkg config (entry + a couple of knobs) no
longer need to commit a separate .pkgrc.json just to customize the build.
Keeps the "pkg owns its schema" property — users still write pkg config
keys (camelCase), we just let them write them inline.

Not masked as a secret — README adds a note warning users against embedding
secrets in the value.

Docs: README gains an "Inline config" subsection with an example; action.yml
+ docs/inputs.md regenerated.

Tests: four new unit tests cover the happy path, mutual exclusion, invalid
JSON, and non-object JSON (string/array/null). 227/227 pass.
Three nits from the Copilot review on PR #13:
- targets.ts cross-compile warnings referenced `fallback-to-source` (kebab,
  like the retired CLI flag); pkg config uses `fallbackToSource` (camelCase).
  Reword both messages to point at the actual config key.
- README "pkg configuration" example was fenced as jsonc with a `//` comment
  and trailing comma but labeled `.pkgrc.json` — .pkgrc.json is strict JSON,
  so a copy/paste would fail. Move the filename into the prose, fence as
  json, drop the comment + trailing comma.

Unit tests + lint green. Matrix bundle rebuilt to pick up the new warning
strings.
# Conflicts:
#	action.yml
#	docs/inputs.md
#	packages/build/action.yml
#	packages/build/dist/index.mjs
#	packages/build/src/main.ts
#	packages/core/src/inputs.ts
#	packages/core/test/unit/inputs.test.ts
#	packages/windows-metadata/dist/index.mjs
Main (PR #12) wired the claude-code-smoke job through the action using
`mode: sea` and `compress-node: Zstd` inputs. Those inputs no longer exist
on this branch, so move both values into the package.json#pkg field that
the fixture-prep step already injects. No behavior change on main; on this
branch the e2e now exercises the pkg-config path the slim input surface
forces users onto.
The merge from main brought in the claude-code-smoke job, which previously
drove pkg via `mode: sea` + `compress-node: Zstd` action inputs. On this
branch those inputs are dropped. The prior merge-conflict resolution moved
both into `package.json#pkg` under the assumption that pkg config would pick
them up. That assumption was wrong:

- `mode: 'sea'` is not a pkg config key — the correct key is `sea: true`.
  (This was a user-visible bug; pkg silently ignored `mode` and ran
  standard-mode, which OOM'd trying to bytecode-compile claude-code's ESM
  bundle on macos-arm64.)
- `compressNode` / `compress` is not a pkg config key at all. `--compress`
  is CLI-only today on @yao-pkg/pkg. No config equivalent exists.

Fix the e2e:
- Write `sea: true` into package.json#pkg so SEA is actually enabled.
- Drop the Zstd request — there is no way to express it without the
  dropped CLI input. The job now exercises SEA + tar.gz; the Zstd branch
  is intentionally deferred until upstream lands config support.
- Rename + retone the job's comment block accordingly.

Also add an upstream-dependency note to STATUS.yaml and a draft issue
body at docs/upstream-pkg-config-issue.md. The draft asks yao-pkg/pkg to
accept the currently-CLI-only build flags (compress, fallbackToSource,
public, publicPackages, options, noBytecode, noDict, debug, signature)
in the config file — that's the piece that makes this PR's scope
defensible for the full input set, not just for SEA.

227 unit tests pass; lint + prettier clean.
Issue is open upstream asking @yao-pkg/pkg to accept the currently-CLI-only
build flags (compress, fallbackToSource, public, publicPackages, options,
noBytecode, noDict, debug, signature) in the config file — the piece that
makes this PR's drop-the-CLI-mirror direction fully viable.

Replace the local draft with the issue URL in STATUS.yaml#upstream-dependency.
@robertsLando
Copy link
Copy Markdown
Member Author

Filed the upstream tracking issue for this PR's scope: yao-pkg/pkg#262 — asks @yao-pkg/pkg to accept the currently-CLI-only build flags (compress, fallbackToSource, public, publicPackages, options, noBytecode, noDict, debug, signature) in the config file. Linked in STATUS.yaml.

Until that lands, this action covers the SEA case (sea: true in pkg config) and everything expressible in pkg config today. The claude-code-smoke e2e has been narrowed accordingly — SEA + tar.gz only, no Zstd — until upstream config support for compress lands.

Comment thread .github/workflows/e2e.yml Outdated
Comment thread .github/workflows/e2e.yml Outdated
pkg v6.19.0 (yao-pkg/pkg#263, closes #262) landed config support for
the CLI-only build flags, and ships the SEA-mode detector fix
(yao-pkg/pkg#268) that caused the prior OOM at bytecode-compile time.

- claude-code-smoke: add `compress: 'Zstd'` alongside `sea: true` in
  package.json#pkg; bump pkg-version input ~6.18.0 → ~6.19.0.
- Default `pkg-version` input bumped ~6.16.0 → ~6.19.0 — the whole
  config-as-source-of-truth direction of this branch requires it.
- Regenerated action.yml / packages/build/action.yml / docs/inputs.md.
- STATUS.yaml: upstream-dependency RESOLVED; Zstd gap dropped;
  e2e-status lists claude-code-smoke; ci-status bumped to 227 tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@robertsLando robertsLando requested a review from Copilot April 24, 2026 13:07
Rebuild packages/{build,windows-metadata}/dist/index.mjs so the
committed bundles match the ~6.19.0 default written into
packages/core/src/inputs.ts. Drift gate caught it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 17 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md Outdated
Comment thread packages/build/src/main.ts Outdated
…ize helper

Address four items from the self-review on PR #13:

- README examples used `mode: "sea"` / `compressNode: "Brotli"` — neither
  exists in @yao-pkg/pkg's config schema. The authoritative FLAG_SPECS in
  lib/config.ts (and the docs-site configuration reference) show `sea: true`
  and `compress: "..."`. Fixed both examples so a copy-paste works.

- `config-inline` carried user-authored JSON but was not masked. Mark the
  spec with `secret: true` so parseInputs registers the raw value via
  core.setSecret — exact-match redaction in logs costs nothing given pkg
  doesn't echo the config blob. README warning softened accordingly (still
  flags the on-disk temp file).

- Extract the config-inline "bytes to disk" step out of the orchestrator
  into `materializePkgConfigInline` in fs-utils, alongside
  `PKG_CONFIG_INLINE_FILENAME`. Four new unit tests cover pass-through,
  both-undefined, inline-writes-and-returns-path, and inline-wins-over-config.
  Orchestrator is now a two-liner that just logs the resulting path.

- BASE_BUILD fixture in pkg-runner.test.ts still pinned pkgVersion to the
  old `~6.16.0` default; bump to `~6.19.0` to match parseInputs.

Regenerated action.yml + docs/inputs.md (secret flag on config-inline
doesn't affect the YAML shape but the description changed). Rebuilt
packages/{build,windows-metadata}/dist/index.mjs.

Tests 231/231 pass (was 227/227), typecheck + lint + prettier green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@robertsLando robertsLando merged commit b5a8624 into main Apr 24, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants